fix(runtime): align URL query serialization with browser behavior#19166
fix(runtime): align URL query serialization with browser behavior#19166bigbigDreamer wants to merge 1 commit intoNervJS:mainfrom
Conversation
Walkthrough重构URL和URLSearchParams实现以保留原始搜索字符串。URL类通过私有字段追踪搜索参数修改状态,未修改时返回原始值。URLSearchParams重构为命名导出类,支持注册变更回调以标记搜索已修改。添加测试验证管道符号处理。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
这个变更应该被明确标记为 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/taro-runtime/src/bom/URLSearchParams.ts (1)
85-87: 可选优化:_setOnChange当前是公开方法。下划线只是命名约定,外部代码(甚至业务侧)通过
url.searchParams._setOnChange(undefined)即可清除回调,导致后续url.search/url.toString()不再反映searchParams的变更,且静默失败、难以排查。如需更严格的隔离,可考虑用模块级Symbol或WeakMap来注册回调,仅在URL.ts中可见。♻️ 参考实现(Symbol 方案)
+export const SET_ON_CHANGE = Symbol('TaroURLSearchParams.setOnChange') + export class TaroURLSearchParams { `#dict` = Object.create(null) `#onChange`?: () => void ... - _setOnChange (onChange?: () => void) { + [SET_ON_CHANGE] (onChange?: () => void) { this.#onChange = onChange }并在
URL.ts的#setSearch中改用this.#search[SET_ON_CHANGE](...)。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/taro-runtime/src/bom/URLSearchParams.ts` around lines 85 - 87, The _setOnChange method on URLSearchParams is currently publicly callable which allows external code to clear the update callback; change the implementation to hide the setter by registering the onChange callback via a module-scoped symbol or WeakMap so only URL.ts can access it (e.g. define a unique SET_ON_CHANGE Symbol or a WeakMap in the module, export nothing, and replace calls to url.searchParams._setOnChange(...) with this.[SET_ON_CHANGE](...) inside URL.ts); update URLSearchParams.ts to remove or make _setOnChange internal and expose the new module-scoped accessor, and update URL.ts to use the symbol/WeakMap key when calling into the searchParams instance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/taro-runtime/src/bom/URLSearchParams.ts`:
- Around line 85-87: The _setOnChange method on URLSearchParams is currently
publicly callable which allows external code to clear the update callback;
change the implementation to hide the setter by registering the onChange
callback via a module-scoped symbol or WeakMap so only URL.ts can access it
(e.g. define a unique SET_ON_CHANGE Symbol or a WeakMap in the module, export
nothing, and replace calls to url.searchParams._setOnChange(...) with
this.[SET_ON_CHANGE](...) inside URL.ts); update URLSearchParams.ts to remove or
make _setOnChange internal and expose the new module-scoped accessor, and update
URL.ts to use the symbol/WeakMap key when calling into the searchParams
instance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9eaf0d1d-cc55-4ff0-8704-afc83aad3c62
📒 Files selected for processing (3)
packages/taro-runtime/src/bom/URL.tspackages/taro-runtime/src/bom/URLSearchParams.tspackages/taro-runtime/tests/location.spec.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #19166 +/- ##
==========================================
+ Coverage 56.07% 56.09% +0.02%
==========================================
Files 447 447
Lines 23454 23469 +15
Branches 5780 5782 +2
==========================================
+ Hits 13151 13166 +15
+ Misses 8448 8442 -6
- Partials 1855 1861 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
这个 PR 做了什么? (简要描述所做更改)
修复小程序端
URL在构造后过早使用URLSearchParams.toString()重写 query 的问题。此前
new URL('https://example.test/?name=|aaa')后,url.search/url.href会被序列化为?name=%7Caaa,与浏览器 WHATWG URL 行为不一致。按 Web 平台 URL 标准,URL query 序列化不应在构造阶段使用application/x-www-form-urlencoded规则;只有在修改searchParams后,才应将参数列表重新序列化并写回 URL。标准依据:
"、#、<、>;special-query percent-encode set 额外包含'。|不在 URL query percent-encode set 中,因此new URL('https://example.test/?name=|aaa').search应保留为?name=|aaa。URLSearchParams的 stringification 使用application/x-www-form-urlencoded序列化规则。|序列化为%7C,因此url.searchParams.toString()应返回name=%7Caaa。URLSearchParams.append/set/delete会触发 update,将参数列表重新序列化并写回关联的 URL query。searchParams后,url.href才应变为https://example.test/?name=%7Caaa&age=18。本 PR 调整了
TaroURL与URLSearchParams的联动逻辑:new URL()后保留原始search表达,避免提前 encode query 参数。searchParams.append/set/delete后再触发 URL query 更新。name=|aaa场景测试,确保url.search/url.href与url.searchParams.toString()分别符合对应序列化规则。这个 PR 是什么类型? (至少选择一个)
这个 PR 涉及以下平台:
Summary by CodeRabbit
发布说明
Bug 修复
测试